Skip to content

fix: 외래키로 인해 물리적 탈퇴가 되지 않는 문제 수정 - #848

Open
whqtker wants to merge 1 commit into
developfrom
whqtker/investigate-withdrawal-logic
Open

whqtker wants to merge 1 commit into
developfrom
whqtker/investigate-withdrawal-logic

Conversation

@whqtker

@whqtker whqtker commented Sep 22, 2026

Copy link
Copy Markdown
Member

관련 이슈

작업 내용

탈퇴 후 30일이 지난 사용자를 삭제하는 UserRemovalScheduler가 외래키 제약으로 인해 매일 밤 롤백되고 있었습니다.

채팅

  • 1:1 채팅방은 참여자 한 명이 사라지면 유지될 수 없으므로 방 전체를 삭제합니다.
    메시지 → 읽음 상태 → 참여자(양쪽) → 방 순으로 정리합니다.
  • 채팅방이 멘토링을 참조하므로 멘토링보다 먼저 삭제하도록 순서를 바꿨습니다.

멘토링

  • 기존에는 멘티로 참여한 멘토링만 삭제했습니다. 멘토로 참여한 멘토링도 함께 삭제합니다.

뉴스

  • 탈퇴자가 쓴 뉴스에 다른 사용자가 누른 좋아요를 함께 삭제합니다.

게시글·채팅 메시지의 소프트 삭제

  • 신고로 가려진(is_deleted = true) 게시글과 메시지는 엔티티 조회 필터에 걸려
    삭제되지 않았습니다. 플래그를 되돌린 뒤 삭제해 JPA cascade가 그대로 동작하게 했습니다.

정지 기록

  • 탈퇴자를 대상으로 한 기록은 삭제합니다.
  • 탈퇴자가 집행한 기록은 다른 사용자의 징계 이력이므로, 집행자 참조만 해제하고 행은 보존합니다.
    이를 위해 user_ban.banned_by를 nullable로 변경했습니다. (V60)

프로필 이미지

  • S3Service.deleteExProfile이 빈 key로 S3를 호출해 예외를 던지던 문제를 막았습니다.
    profile_image_url이 null이거나 빈 문자열인 계정에서 발생합니다.

특이 사항

리뷰 요구사항 (선택)

탈퇴 후 30일이 지난 사용자를 삭제하는 스케줄러가 외래키 제약에 걸려
매일 롤백되고 있었다. 삭제 목록에서 빠져 있던 데이터를 채우고 순서를 바로잡는다.

- 1:1 채팅방은 참여자가 사라지면 유지될 수 없으므로 방 전체를 삭제한다
- 채팅방이 멘토링을 참조하므로 멘토링보다 먼저 삭제한다
- 멘토로 참여한 멘토링과 탈퇴자가 쓴 뉴스의 좋아요를 함께 삭제한다
- 신고로 가려진 게시글과 메시지는 조회 필터에 걸리므로 플래그를 되돌린 뒤 삭제한다
- 정지 기록은 대상 행만 삭제하고, 집행자 참조는 해제해 다른 사용자의 이력을 보존한다
- 프로필 이미지 key 가 비어 있으면 S3 호출을 건너뛴다

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@whqtker whqtker self-assigned this Sep 22, 2026
@whqtker whqtker added the 버그 Something isn't working label Sep 22, 2026
@whqtker whqtker added the 최종 리뷰 최소 1명 필수 label Sep 22, 2026
@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 7bbbda38-1d32-437e-b438-7c9872accfed

📥 Commits

Reviewing files that changed from the base of the PR and between 7f09368 and 6a430c8.

📒 Files selected for processing (13)
  • src/main/java/com/example/solidconnection/chat/repository/ChatMessageRepository.java
  • src/main/java/com/example/solidconnection/chat/repository/ChatParticipantRepository.java
  • src/main/java/com/example/solidconnection/chat/repository/ChatReadStatusRepository.java
  • src/main/java/com/example/solidconnection/community/post/repository/PostRepository.java
  • src/main/java/com/example/solidconnection/mentor/repository/MentorRepository.java
  • src/main/java/com/example/solidconnection/mentor/repository/MentoringRepository.java
  • src/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.java
  • src/main/java/com/example/solidconnection/news/repository/NewsRepository.java
  • src/main/java/com/example/solidconnection/s3/service/S3Service.java
  • src/main/java/com/example/solidconnection/scheduler/UserRemovalScheduler.java
  • src/main/java/com/example/solidconnection/siteuser/domain/UserBan.java
  • src/main/java/com/example/solidconnection/siteuser/repository/UserBanRepository.java
  • src/main/resources/db/migration/V60__alter_user_ban_banned_by_nullable.sql

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

탈퇴 사용자 삭제 흐름이 연관 데이터 유형별 헬퍼 메서드를 사용하도록 변경되었습니다. 뉴스 좋아요, 신고로 표시된 게시물, 1:1 채팅 데이터, 멘토링, 제재 참조를 삭제 순서에 맞게 정리합니다. banned_by는 NULL을 허용하도록 변경되었습니다. 프로필 이미지 키가 없으면 S3 삭제를 실행하지 않습니다.

Priority: ⬆️ High

Estimated code review effort: 4 (Complex) | ~45 minutes

  1. 연관 데이터 삭제 메서드 추가
    채팅 메시지와 읽음 상태를 채팅방 ID 기준으로 삭제합니다.
    뉴스 ID와 멘토 ID 목록을 조회하고 관련 레코드를 일괄 삭제합니다.

  2. 탈퇴 사용자 삭제 흐름 변경
    UserRemovalScheduler가 뉴스, 게시물, 채팅방, 멘토링, 제재 데이터를 순서대로 정리합니다.
    채팅방은 메시지, 읽음 상태, 참여자, 채팅방을 모두 삭제합니다.

  3. 제재 참조 정리
    사용자가 피제재자인 레코드는 삭제합니다.
    사용자가 제재를 수행하거나 해제한 레코드는 해당 참조를 NULL로 변경합니다.

  4. 프로필 이미지 삭제 가드 추가
    프로필 이미지 키가 NULL 또는 공백이면 deleteFile을 호출하지 않습니다.

Merge Risk: ⚪ Minimal · up to 6a430

The scheduler now removes dependent data before deleting departed users and safely skips empty profile-image keys. No concrete merge-blocking correctness or availability risk is evidenced.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 12 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 제목은 외래키 제약으로 물리적 탈퇴가 실패하는 문제를 수정한다는 PR의 핵심 변경을 명확하게 설명합니다.
Description check ✅ Passed 관련 이슈와 작업 내용을 포함하며, 채팅·멘토링·뉴스·소프트 삭제·정지 기록·프로필 이미지 변경 사항을 기능 단위로 설명합니다. 선택 항목인 리뷰 요구사항과 특이 사항은 비어 있지만 설명의 핵심 정보는 충분합니다.
Linked Issues check ✅ Passed 1. 채팅 정리: UserRemovalScheduler.deleteChatRooms가 메시지의 삭제 플래그를 되돌린 뒤 메시지, 읽음 상태, 참여자, 채팅방을 순서대로 삭제합니다. 채팅방 삭제가 멘토링 삭제보다 먼저 실행됩니다. [#844]
2. 멘토링과 콘텐츠 정리: 멘티 및 멘토 기준 멘토링을 삭제합니다. 탈퇴자 뉴스의 좋아요를 뉴스 삭제 전에 삭제…
Out of Scope Changes check ✅ Passed 변경은 #844의 외래키 삭제 순서, 관련 자식 데이터 삭제, 정지 참조 해제, 프로필 이미지 null 처리에 집중됩니다. 리포지토리 메서드와 V60 마이그레이션은 이 동작을 지원하는 변경입니다. 별도의 기능 변경이나 관련 없는 변경은 확인되지 않습니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 17.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 35 functions across 12 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a430c8465

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

private Long bannedUserId;

@Column(name = "banned_by", nullable = false)
@Column(name = "banned_by")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Declare the nullable mapping explicitly

Because this change intentionally makes banned_by nullable, declare that contract as @Column(name = "banned_by", nullable = true) rather than relying on the annotation default. The repository convention requires entity columns to state nullability explicitly, so the current mapping no longer documents the schema change consistently.

AGENTS.md reference: AGENTS.md:L301-L305

Useful? React with 👍 / 👎.

return;
}
chatMessageRepository.unmarkDeletedByChatRoomIdIn(chatRoomIds);
chatMessageRepository.deleteAllByChatRoomIdIn(chatRoomIds);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Delete chat attachment objects before dropping their rows

When a deleted room contains files uploaded through the chat upload endpoint, this call cascades removal of the ChatAttachment database rows but never deletes the objects referenced by ChatAttachment.url from S3. Those keys become unreachable after the rows are removed and accumulate permanently; collect and delete the attachment objects before deleting the messages.

Useful? React with 👍 / 👎.

@Hexeong Hexeong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하십니다~ 의견 남게두겠습니다!

* - 채팅방이 멘토링을 참조하므로 멘토링보다 먼저 삭제한다.
* */
private void deleteChatRooms(long siteUserId) {
List<Long> chatRoomIds = chatParticipantRepository.findAllChatRoomIdsBySiteUserId(siteUserId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 ChatRoom의 스키마 상 isGroup 컬럼이 존재합니다! 하지만 현재 findAllChatRoomIdsBySiteUserId가 isGroup 여부를 구분하지 않아서, 탈퇴자가 속한 채팅방은 1:1이든 그룹이든 전부 여기서 삭제 대상이 됩니다.

PR 설명에는 "1:1 채팅방은 참여자가 사라지면 유지될 수 없으므로 삭제"라고 되어 있는데 코드에는 그 조건이 명시돼 있지 않네요. 지금은 그룹 채팅방을 생성하는 경로가 없어 실제 영향은 없지만, ChatRoom.isGroup이 이미 존재하고 ChatService에서도 분기 처리하는 걸 보면 나중에 그룹 채팅에 대해서도 대비가 되게 구현하는 게 좋아보입니다.

그때 멤버 한 명 탈퇴로 다른 멤버들의 방 전체가 사라지는 회귀를 막기 위해, isGroup = false인 방만 대상으로 삼도록 조건을 걸거나 최소한 의도를 주석/TODO로 남겨두면 좋을 것 같습니다!

chatMessageRepository.deleteAllByChatRoomIdIn(chatRoomIds);
chatReadStatusRepository.deleteAllByChatRoomIdIn(chatRoomIds);
chatParticipantRepository.deleteAllByChatRoomIdIn(chatRoomIds);
chatRoomRepository.deleteAllById(chatRoomIds);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1:1 채팅방을 통째로 삭제하면, 탈퇴하지 않은 상대방 입장에서는 아무 통보 없이 대화 기록이 사라지게 됩니다!

참여자 한 명이 빠지면 방을 유지하기 어렵다는 제약 자체는 이해되지만, 이게 기획 쪽과 합의된 정책인지 확인이 필요해 보입니다. 필요하면 상대방 메시지는 보존하고 탈퇴한 참여자 정보만 "알 수 없음" 등으로 치환하는 방식도 고려해볼 수 있을 것 같습니다.

List<Long> chatParticipantIds = chatParticipantRepository.findAllIdsBySiteUserId(siteUserId);
chatReadStatusRepository.deleteAllByChatParticipantIdIn(chatParticipantIds);
chatParticipantRepository.deleteAllBySiteUserId(siteUserId);
reportRepository.deleteAllByReporterId(siteUserId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteAllByReporterId는 이 사용자가 "신고한" 기록만 지우는데, 이 사용자가 "신고당한" 기록(report.reported_id = siteUserId)이나 이 사용자의 게시글/채팅메시지를 대상(target_id)으로 한 신고 기록은 그대로 남아 고아 데이터가 발생할 수 있을 것 같습니다!

target_id/reported_id에는 FK가 없어서(ReportRepository, V25__create_report_table.sql 확인) 삭제 자체가 실패하지는 않지만, 대상이 사라진 report 행이 고아 데이터로 계속 쌓이게 됩니다. 이번 PR 범위 밖일 수 있지만 후속으로 reportedId/targetId 기준 정리도 필요해 보입니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

버그 Something isn't working 최종 리뷰 최소 1명 필수

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: 탈퇴 사용자 물리 삭제가 외래키 제약으로 전면 실패하는 문제를 수정한다.

2 participants